src/backend/chat/evals/README.md (2)

9-23: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to the fenced code block.

The code fence at line 9 still lacks a language identifier, triggering markdownlint MD040. Adding text or plaintext will resolve the linting warning.

📝 Proposed fix

-```
+```text
 chat/evals/
 ├── configs/
 │   ├── __init__.py          # REGISTRY — maps dataset name → EvalConfig

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/README.md` around lines 9 - 23, The fenced code block
in src/backend/chat/evals/README.md is missing a language identifier (triggering
MD040); update the opening triple-backtick of the directory tree block to
include a language token such as ```text or ```plaintext so the block becomes a
proper fenced code block with a language identifier; locate the block showing
"chat/evals/" and change its opening fence accordingly.

---

29-29: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify that URLs from user input are also allowed.

The description states URLs must come "only from tool output," but if the evaluator also accepts URLs present in user_message (as flagged in previous review), this description is misleading. Consider updating to "only uses URLs from tool output or user input" to accurately reflect the evaluator behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/README.md` at line 29, Update the README description
for the `url_hallucination` check to reflect that allowed URLs may come from
both tool outputs and user input: change the phrase "only uses URLs from tool
output" to "only uses URLs from tool output or user input" and ensure the
`UrlRegexEvaluator` and `LLMJudge` references remain intact so readers know
those components perform regex and semantic checks respectively (refer to
`url_hallucination`, `UrlRegexEvaluator`, and `LLMJudge` in the same line).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/chat/evals/compare.py (2)

158-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Split report formatting into smaller render helpers.

SonarCloud reports format_comparison above the allowed cognitive-complexity threshold. Extract dataset header rendering, case-change rendering, and score/reason rendering into helpers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/compare.py` around lines 158 - 226, The
format_comparison function has too much nested rendering logic and exceeds the
cognitive-complexity limit. Refactor it by extracting the dataset header/status
lines, the case-change block, and the score/reason rendering into small helper
functions near format_comparison in compare.py, then have format_comparison
orchestrate those helpers while preserving the current output format.

Source: Linters/SAST tools

---

76-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Split compare_runs to satisfy the failing complexity gate.

SonarCloud reports this function above the allowed cognitive-complexity threshold. Extracting dataset lookup/warning handling and per-case delta construction into helpers should unblock the check without changing behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/compare.py` around lines 76 - 143, The compare_runs
function is exceeding the cognitive-complexity limit; split its dataset-level
warning/lookup logic and case-delta building into small helpers. Refactor around
compare_runs, _case_map, DatasetComparison, and CaseChange so the main function
only orchestrates comparison while preserving the same behavior and warnings.

Source: Linters/SAST tools

src/backend/chat/management/commands/run_evals.py (1)

20-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove commented-out config lines.

These commented settings overrides are dead artifacts and flagged by static analysis.

🧹 Proposed fix

-# settings.LLM_DEFAULT_MODEL_HRID = "etalab-plateform-mistral-medium-2508"
-# settings.LANGFUSE_ENABLED = False
-
-

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/management/commands/run_evals.py` around lines 20 - 21,
Remove the commented-out settings overrides from run_evals.py; these are dead
artifacts and should not remain in the command module. Delete the commented
lines near the settings references in run_evals and keep the file focused on
active configuration logic only.

Source: Linters/SAST tools

src/backend/chat/tests/evals/test_production_agent.py (1)

83-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test depends on, and masks, the process-global caching behavior.

This test asserts identity-of-cached-state across two builder calls, which only passes because of the module-level globals in production_agent.py. If those globals are reset for isolation (see the related comment on production_agent.py), this test would need both builds to run within the same un-reset scope. Worth making the dependency explicit (e.g., build both inside one logical step) so the test intent survives an isolation fix.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/tests/evals/test_production_agent.py` around lines 83 - 90,
This test is relying on the process-global cache in
build_production_agent_service, so make the dependency explicit by grouping the
two service builds in one un-reset scope or shared step. Update
test_eval_session_reuses_same_user_and_conversation to clearly exercise the same
cached session behavior for both service_a and service_b, using the
build_production_agent_service helper and the user/conversation assertions, so
it still reflects the intended reuse if module-level globals in
production_agent.py are later isolated or reset.

src/backend/chat/evals/configs/faithfulness_rag.py (1)

82-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

_CURRENT_CHUNKS is set but never reset between cases.

The contextvar is overwritten on each run_agent call, so correctness relies on every case setting it before the stub reads it. This holds today (sequential, max_concurrency=1), but if a case ever has tool_output=None/empty while the tool still fires, it will silently fall back to the previous case's chunks only if set is skipped — here set(None) is always called, so the fallback string handles it. No bug, but a try/finally reset (or ContextVar.reset via the token) would make this robust against future concurrency changes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/configs/faithfulness_rag.py` around lines 82 - 86, The
run_agent helper in faithfulness_rag.py sets _CURRENT_CHUNKS for each eval case
but never restores the previous ContextVar state, so make it robust by capturing
the token from _CURRENT_CHUNKS.set(...) and resetting it in a try/finally around
agent.run. Update the run_agent function to use ContextVar.reset via the token
so the stub state cannot leak across future concurrent or failed runs.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/chat/evals/compare.py (1)

112-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Reduce cognitive complexity flagged by SonarCloud (compare_runs 28, format_comparison 42).

Both functions exceed the project's allowed cognitive complexity and are failing the SonarCloud gate. Consider extracting the missing-after-dataset branch (Lines 130-152) and the per-case diff (Lines 174-206) into helpers, and splitting format_comparison's dataset/case rendering into smaller functions, to bring complexity under threshold without behavior change.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/compare.py` around lines 112 - 210, The SonarCloud
gate is failing because compare_runs and format_comparison are too complex.
Refactor compare_runs by extracting the missing-after-dataset handling and the
per-case comparison/diff logic into small helpers, using the existing
compare_runs, _case_map, and _missing_after_case_change flow as anchors. Then
split format_comparison into separate dataset-level and case-level rendering
helpers so each function’s branching is reduced without changing behavior.

Source: Linters/SAST tools

src/backend/chat/evals/storage.py (3)

41-54: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Same invalid except syntax repeated in _run_git.

Identical issue as _git_root — apply the same tuple fix here.

🐛 Proposed fix

-    except subprocess.CalledProcessError, FileNotFoundError:
+    except (subprocess.CalledProcessError, FileNotFoundError):
         return None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/storage.py` around lines 41 - 54, The _run_git helper
still uses invalid multi-exception syntax in its except block, matching the same
issue as _git_root. Update the exception handling in _run_git to catch both
subprocess.CalledProcessError and FileNotFoundError using the correct tuple
form, keeping the fallback return None behavior unchanged. Use the _run_git
function name to locate the fix.

---

25-38: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

SyntaxError: invalid Python 2 multi-exception syntax breaks module import.

except subprocess.CalledProcessError, FileNotFoundError: is invalid in Python 3 — must be a tuple. This raises SyntaxError at import time, breaking the entire chat.evals.storage module (and by extension run_evals, save_run). A prior review flagged this same issue and it was reported as addressed, but the code shown here still has the bug — please re-verify the fix landed on this branch.

🐛 Proposed fix

-    except subprocess.CalledProcessError, FileNotFoundError:
+    except (subprocess.CalledProcessError, FileNotFoundError):
         return None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/storage.py` around lines 25 - 38, The `_git_root`
helper still uses invalid Python 2 exception syntax, which prevents
`chat.evals.storage` from importing. Update the `except` clause in `_git_root`
to use Python 3 multi-exception syntax with a tuple, keeping the existing
fallback behavior that returns `None` on `subprocess.CalledProcessError` or
`FileNotFoundError`. Verify the fix in the `storage.py` module so `run_evals`
and `save_run` can import successfully.

---

94-98: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Run-ID collisions still possible at second precision.

make_run_id still uses second-precision timestamps; two runs within the same second (same commit or both nogit) generate identical IDs, so save_run overwrites the earlier run file. This was flagged previously and remains unresolved.

🛠️ Proposed fix

 def make_run_id(git_short: str | None) -> str:
     """Generate a unique run ID based on timestamp and Git commit short."""
-    timestamp = datetime.now(UTC).strftime("%Y-%m-%dT%H-%M-%SZ")
+    timestamp = datetime.now(UTC).strftime("%Y-%m-%dT%H-%M-%S-%fZ")
     suffix = git_short or "nogit"
     return f"{timestamp}_{suffix}"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/storage.py` around lines 94 - 98, make_run_id still
generates run IDs at second precision, so save_run can overwrite a previous run
when two executions happen in the same second; update make_run_id to include
finer-grained uniqueness (for example, sub-second time and/or a random or
monotonic component) while keeping the existing git_short/nogit suffix, and
ensure save_run continues to use the returned ID so each run file name stays
unique.

src/backend/chat/management/commands/reset_evals.py (1)

5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Constants imported by value can't be patched via chat.evals.storage.

BASELINES_DIR, DASHBOARD_DIR, and RUNS_DIR are imported directly (from chat.evals.storage import ...), binding local names in this module. Per pytest's monkeypatch guidance, tests must patch the name where it's used, i.e. chat.management.commands.reset_evals.RUNS_DIR, not chat.evals.storage.RUNS_DIR — patching the latter (as done in test_storage.py) would have no effect here. Referencing the storage module directly (storage.RUNS_DIR) instead of importing the names would avoid this footgun and keep a single source of truth.

♻️ Suggested refactor

-from chat.evals.storage import BASELINES_DIR, DASHBOARD_DIR, RUNS_DIR, reset_eval_artifacts
+from chat.evals import storage
+from chat.evals.storage import reset_eval_artifacts

Then reference storage.RUNS_DIR, storage.BASELINES_DIR, storage.DASHBOARD_DIR throughout.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/management/commands/reset_evals.py` at line 5, The command
module is importing BASELINES_DIR, DASHBOARD_DIR, and RUNS_DIR by value from
chat.evals.storage, which prevents tests from monkeypatching the source module
and creates a separate binding. Update reset_evals to reference the storage
module directly (for example via chat.evals.storage or an alias) and use
storage.RUNS_DIR, storage.BASELINES_DIR, and storage.DASHBOARD_DIR throughout,
keeping reset_eval_artifacts and the command logic on a single source of truth.

src/backend/chat/management/commands/run_evals.py (1)

20-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove leftover commented-out debug lines.

🧹 Proposed fix

-# settings.LLM_DEFAULT_MODEL_HRID = "etalab-plateform-mistral-medium-2508"
-# settings.LANGFUSE_ENABLED = False

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/management/commands/run_evals.py` around lines 20 - 21,
Remove the leftover commented-out debug settings in the run_evals command by
deleting the two inactive settings lines near the top of run_evals.py; keep the
command clean and only leave the active logic in the module.

Source: Linters/SAST tools

src/backend/chat/evals/dashboard/template.html (1)

786-789: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant nested ternary always resolves to the same value.

Every branch of kind === "regression" || kind === "partial_down" ? "row-changed" : kind === "improvement" || kind === "partial_up" ? "row-changed" : "row-changed" returns "row-changed", so this is equivalent to a single fallback value. SonarCloud also flags this (line 788: conditional returns the same value regardless of condition).

♻️ Proposed simplification

-              const rowClass = kind === "stable" ? (a.passed ? "row-pass" : "row-fail")
-                : kind === "regression" || kind === "partial_down" ? "row-changed"
-                : kind === "improvement" || kind === "partial_up" ? "row-changed" : "row-changed";
+              const rowClass = kind === "stable" ? (a.passed ? "row-pass" : "row-fail") : "row-changed";

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/dashboard/template.html` around lines 786 - 789, The
rowClass ternary in template.html is redundant because every non-stable branch
returns the same "row-changed" value. Simplify the conditional around rowClass
in the dashboard template so it only checks for "stable" versus the fallback,
using the existing rowClass and caseDelta logic as the anchor points.

Source: Linters/SAST tools

src/backend/chat/tests/evals/test_production_agent.py (1)

36-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared fixtures into a conftest.py.

reset_eval_session_cache_fixture and ai_settings_fixture are duplicated verbatim in test_tool_selection.py. Moving them to a conftest.py under chat/tests/evals/ would avoid drift as more eval test modules are added.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/tests/evals/test_production_agent.py` around lines 36 - 66,
The two autouse fixtures in this test module are duplicated in another eval test
file, so they should be centralized. Move reset_eval_session_cache_fixture and
ai_settings_fixture into a conftest.py under chat/tests/evals/, keeping their
behavior the same and ensuring the test modules still pick them up automatically
via pytest fixture discovery.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/chat/evals/datasets/tool_selection.yaml (1)

8-631: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider YAML anchors to reduce evaluator boilerplate.

Nearly every case repeats the identical HasMatchingSpan/HasNoMatchingSpan block shape differing only in gen_ai.tool.name and evaluation_name. YAML anchors/aliases (or merge keys) could cut this duplication significantly and make future edits (e.g., adding a new tool name) less error-prone.

♻️ Example using anchors

.web_search_span: &web_search_span
  query:
    has_attributes:
      gen_ai.tool.name: "web_search"

cases:
  - name: news_recent_ai
    ...
    evaluators:
      - HasMatchingSpan:
          <<: *web_search_span
          evaluation_name: called_web_search

This is a nice-to-have; not blocking given the file is data-only and each case is still easy to read individually.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/datasets/tool_selection.yaml` around lines 8 - 631,
The evaluator blocks in this YAML are highly repetitive, so refactor the
repeated HasMatchingSpan and HasNoMatchingSpan query shapes using YAML
anchors/aliases or merge keys. Define reusable anchored query templates for each
tool name in the dataset, then reference them across the cases while keeping
each case’s evaluation_name explicit; this will reduce boilerplate and make
updates in tool_selection.yaml easier to maintain.

src/backend/chat/evals/dashboard.py (2)

70-78: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Minor: direct dict indexing for run_id could raise KeyError.

entry["run_id"] is used via direct indexing in the filter (line 71) and loop (line 78) rather than .get("run_id"). If a legacy/malformed index entry lacks this key, dashboard generation will crash instead of degrading gracefully.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/dashboard.py` around lines 70 - 78, The dashboard
entry handling in `dashboard.py` uses direct `entry["run_id"]` access in both
the `kept_entries` filter and the `for entry in kept_entries` loop, which can
raise `KeyError` for malformed records. Update the logic around `kept_entries`
and `resolve_run` to read the run id safely with `.get("run_id")`, and skip or
ignore entries that do not have a valid run id so dashboard generation continues
gracefully.

---

36-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider isolating per-file YAML parse failures.

yaml.safe_load(raw) has no error handling; a single malformed or hand-edited dataset YAML file will raise and abort load_dataset_catalog entirely, breaking dashboard generation for all datasets rather than just the offending one.

♻️ Proposed fix

     for yaml_path in sorted(root.glob("*.yaml")):
         raw = yaml_path.read_text(encoding="utf-8")
-        data = yaml.safe_load(raw) or {}
+        try:
+            data = yaml.safe_load(raw) or {}
+        except yaml.YAMLError:
+            continue

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/dashboard.py` around lines 36 - 53,
`load_dataset_catalog` currently lets a malformed YAML file from
`yaml.safe_load` abort the whole catalog load. Wrap the per-file parse in error
handling inside `load_dataset_catalog`, and if parsing fails, skip that
yaml_path while continuing to process the remaining files. Keep the existing
behavior for valid files, and use the existing symbols `load_dataset_catalog`,
`yaml.safe_load`, and `yaml_path` to localize the fix.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/chat/evals/storage.py (1)

27-37: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Fix the exception syntax in src/backend/chat/evals/storage.py

Python 3 rejects except subprocess.CalledProcessError, FileNotFoundError: as a syntax error, so the module can’t import. Update both occurrences to tuple syntax:

Proposed fix

-    except subprocess.CalledProcessError, FileNotFoundError:
+    except (subprocess.CalledProcessError, FileNotFoundError):
         return None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/storage.py` around lines 27 - 37, The exception
handling in storage.py uses Python 2 comma syntax, which breaks import in Python
3. Update both try/except blocks around the git root lookup and any similar
handling in the same module to use tuple-based except syntax with
subprocess.CalledProcessError and FileNotFoundError, so the storage helpers can
import and run correctly.

src/backend/chat/evals/dashboard/template.html (1)

730-737: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Nested ternary flagged by SonarCloud.

deltaHtml/fillB use nested ternaries; not CI-blocking (warning, not failure) but worth flattening for readability, consistent with the earlier cognitive-complexity cleanup in this file.

♻️ Suggested refactor

-      const deltaHtml = pp == null
-        ? '<span class="muted">—</span>'
-        : pp === 0
-          ? '<span class="muted">=</span>'
-          : `<span class="${pp > 0 ? "delta-pos" : "delta-neg"}">${pp > 0 ? "+" : ""}${pp}pp</span>`;
+      let deltaHtml = '<span class="muted">—</span>';
+      if (pp !== null) {
+        deltaHtml = pp === 0
+          ? '<span class="muted">=</span>'
+          : `<span class="${pp > 0 ? "delta-pos" : "delta-neg"}">${pp > 0 ? "+" : ""}${pp}pp</span>`;
+      }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/dashboard/template.html` around lines 730 - 737, The
summaryRow function contains nested ternaries in deltaHtml and fillB that should
be flattened for readability. Refactor the logic in summaryRow to use simple
if/else branches or small helper variables instead of chained ternary
expressions, keeping the same behavior for null, zero, positive, and negative
values.

Source: Linters/SAST tools

src/backend/chat/tests/evals/test_storage.py (1)

37-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding coverage for multiple named baselines.

These tests only exercise a single baseline name. Once the is_baseline flag bug in storage.py's set_baseline (clobbering other named baselines' flags) is fixed, a regression test asserting is_baseline stays True for a previously-set baseline after promoting a different one would guard against reintroduction.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/tests/evals/test_storage.py` around lines 37 - 86, Add a
regression test around set_baseline in chat.evals.storage that covers promoting
multiple named baselines. Reuse the existing
test_set_baseline_copies_run_snapshot setup, then set one baseline, promote a
different baseline, and assert the first baseline’s is_baseline flag remains
True while the new one is also marked correctly. Locate the behavior in
set_baseline and verify the index.json state after each promotion to catch the
clobbering bug.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/chat/tests/evals/test_compare.py (1)

115-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the partial_down branch too.

partial_up and partial_down are the two halves of the same conditional in compare.py, and partial_down is the one that signals degradation. A mirrored case is nearly free here.

♻️ Suggested companion test

+def test_kind_partial_down_when_both_fail_and_rate_drops():
+    """Both runs fail but the repeat pass rate drops: kind is partial_down."""
+    partial_before = {
+        "name": "p",
+        "passed": False,
+        "pass_rate": 0.8,
+        "avg_scores": {},
+        "reasons": {},
+    }
+    partial_after = {**partial_before, "pass_rate": 0.2}
+    before = _run(run_id="before", datasets={"ds": _dataset(pass_rate=0.0, cases=[partial_before])})
+    after = _run(run_id="after", datasets={"ds": _dataset(pass_rate=0.0, cases=[partial_after])})
+
+    payload = compare_runs(before, after).to_payload()
+
+    kinds = {c["name"]: c["kind"] for c in payload["datasets"]["ds"]["case_changes"]}
+    assert kinds == {"p": "partial_down"}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/tests/evals/test_compare.py` around lines 115 - 131, Add a
mirrored regression test alongside
test_kind_partial_up_when_both_fail_but_rate_improves for the partial_down
branch in compare_runs: keep both runs failing, lower the case pass_rate in the
after result, and assert the resulting case_changes entry for "p" has kind
"partial_down".

src/backend/chat/evals/dashboard/template.html (1)

716-723: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract nested ternary in summaryRow (SonarCloud flag).

deltaHtml's nested ternary is still flagged by SonarCloud. Extracting to an if/else or a small helper improves readability and clears the CI warning.

♻️ Proposed refactor

     function summaryRow(label, rateA, rateB, { isTotal = false } = {}) {
       const pp = rateA != null && rateB != null ? Math.round((rateB - rateA) * 100) : null;
-      const deltaHtml = pp == null
-        ? '<span class="muted">—</span>'
-        : pp === 0
-          ? '<span class="muted">=</span>'
-          : `<span class="${pp > 0 ? "delta-pos" : "delta-neg"}">${pp > 0 ? "+" : ""}${pp}pp</span>`;
+      let deltaHtml;
+      if (pp == null) {
+        deltaHtml = '<span class="muted">—</span>';
+      } else if (pp === 0) {
+        deltaHtml = '<span class="muted">=</span>';
+      } else {
+        deltaHtml = `<span class="${pp > 0 ? "delta-pos" : "delta-neg"}">${pp > 0 ? "+" : ""}${pp}pp</span>`;
+      }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/dashboard/template.html` around lines 716 - 723,
Refactor the nested ternary used to build deltaHtml inside summaryRow into
explicit if/else logic or a small helper, preserving the existing null, zero,
positive, and negative output markup and classes while eliminating SonarCloud’s
nested-ternary warning.

Source: Linters/SAST tools

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/chat/evals/storage.py (1)

28-37: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Use Python 3 exception tuple syntax.

storage.py is still parsed as Python 3, so except subprocess.CalledProcessError, FileNotFoundError: is invalid syntax at both lines 36 and 52 and prevents the module from importing.

🐛 Proposed fix (apply to both lines 36 and 52)

-    except subprocess.CalledProcessError, FileNotFoundError:
+    except (subprocess.CalledProcessError, FileNotFoundError):
         return None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/storage.py` around lines 28 - 37, Update both
exception handlers in the storage path-resolution logic to use Python 3 tuple
syntax, replacing the comma-separated exception form in the relevant try/except
blocks while preserving the existing return None behavior.

src/backend/chat/tests/agents/test_albert_models.py (1)

283-310: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the non-dict tool_call defensive branch.

_validate_completion guards against tool_call entries that aren't dicts (if not isinstance(tool_call, dict): continue in albert_models.py), but no test here exercises that path. Consider a case where choices[0].message.tool_calls contains a non-dict entry to lock in that defensive behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/tests/agents/test_albert_models.py` around lines 283 - 310,
Add a focused test alongside the existing _validate_completion tests that builds
a completion whose choices[0].message.tool_calls contains a non-dict entry, then
calls albert_model._validate_completion and verifies validation completes
without error while preserving or skipping that entry according to the existing
defensive behavior. Reuse _make_malformed_chat_completion and the established
completion/message fixtures or symbols rather than adding unrelated coverage.

src/backend/chat/evals/dashboard/template.html (1)

76-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Low-contrast header subtitle text.

.site-header-subtitle uses #94a3b8 on the dark .site-header gradient background, flagged by SonarCloud as failing minimum contrast.

💡 Proposed fix

     .site-header-subtitle {
       margin: .45rem 0 0;
       max-width: 36rem;
       font-size: .92rem;
       line-height: 1.45;
-      color: `#94a3b8`;
+      color: `#cbd5e1`;
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/dashboard/template.html` at line 76, Update the
.site-header-subtitle color from `#94a3b8` to a lighter color that meets
SonarCloud’s minimum contrast requirement against the dark .site-header
gradient, while preserving the existing header styling.

Source: Linters/SAST tools

src/backend/chat/providers/albert_models.py (1)

78-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the tool-call normalization loop to satisfy the SonarCloud complexity gate.

Cognitive complexity is 16 vs the 15 allowed (CI gate failure). Pulling the inner for tool_call in ... loop into a small helper keeps _validate_completion as orchestration only.

♻️ Proposed refactor

+    `@staticmethod`
+    def _normalize_tool_call_types(choices: list[dict]) -> None:
+        for choice in choices:
+            for tool_call in (choice.get("message") or {}).get("tool_calls") or []:
+                if not isinstance(tool_call, dict):
+                    continue
+                is_custom = tool_call.get("type") == "custom" and "custom" in tool_call
+                if not is_custom and tool_call.get("type") != "function":
+                    tool_call["type"] = "function"
+
     def _validate_completion(self, response: chat.ChatCompletion) -> _ChatCompletion:
         ...
         if not isinstance(data.get("choices"), list):
             data["choices"] = []
-
-        for choice in data.get("choices") or []:
-            for tool_call in (choice.get("message") or {}).get("tool_calls") or []:
-                if not isinstance(tool_call, dict):
-                    continue
-                is_custom = tool_call.get("type") == "custom" and "custom" in tool_call
-                if not is_custom and tool_call.get("type") != "function":
-                    tool_call["type"] = "function"
-
+        self._normalize_tool_call_types(data["choices"])
         return _ChatCompletion.model_validate(data)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/providers/albert_models.py` around lines 78 - 108, Extract
the inner tool-call normalization loop from _validate_completion into a small
dedicated helper that accepts the relevant message or tool-call collection and
preserves the existing custom/function type rules. Have _validate_completion
delegate to this helper while retaining only response normalization and
orchestration, reducing its cognitive complexity without changing behavior.

Source: Linters/SAST tools

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/chat/evals/storage.py (2)

36-37: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Python 2 except syntax — module fails to import.

except subprocess.CalledProcessError, FileNotFoundError: is a hard SyntaxError in Python 3, so storage.py (and every command importing it) cannot load. Same at Line 52. This was previously reported as fixed, but the supplied code still has it.

🐛 Proposed fix (apply to both lines 36 and 52)

-    except subprocess.CalledProcessError, FileNotFoundError:
+    except (subprocess.CalledProcessError, FileNotFoundError):
         return None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/storage.py` around lines 36 - 37, Update both
exception handlers in storage.py to use Python 3 tuple syntax for catching
subprocess.CalledProcessError and FileNotFoundError, preserving their existing
return behavior so the module imports successfully.

---

93-97: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Run IDs still collide at second precision.

Two runs in the same second on the same commit produce the same run_id, so save_run overwrites the earlier artifact and the index entry is replaced. Add sub-second precision or a short random suffix.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/storage.py` around lines 93 - 97, Update make_run_id
so generated IDs remain unique for runs sharing the same second and Git commit,
by adding sub-second timestamp precision or a short random suffix while
preserving the existing timestamp and git_short components.

src/backend/chat/management/commands/reset_evals.py (1)

49-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Dry-run still reports index.json as deleted.

reset_eval_artifacts excludes index.json from unlinking (storage.py Line 313) and only clears its contents in place, and Line 55 also appends it even when it doesn't exist.

🩹 Proposed fix

-            targets.append(str((RUNS_DIR / "index.json").relative_to(RUNS_DIR.parent)))
+            index_path = RUNS_DIR / "index.json"
+            if index_path.exists():
+                targets.append(f"{index_path.relative_to(RUNS_DIR.parent)} (reset, not deleted)")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/management/commands/reset_evals.py` around lines 49 - 55,
Update the target collection in reset_eval_artifacts so dry-run output matches
the actual cleanup behavior: exclude index.json from the deletion targets and
only include it when it exists and is being cleared in place. Preserve reporting
of other matching run JSON files.

src/backend/chat/management/commands/run_evals.py (2)

86-91: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Incomplete flag-combination validation in handle. Only --save + --case is rejected; the other two invalid combinations involving --dataset slip through and either persist a partial run or abort a legitimate single-case run.

• src/backend/chat/management/commands/run_evals.py#L86-L91: extend the guard to also reject --save with --dataset (partial record → every omitted dataset scores as a coverage gap against a full baseline), and to reject --case without --dataset.
• src/backend/chat/management/commands/run_evals.py#L106: with that guard in place, configs is a single dataset whenever --case is set, so _load_dataset no longer aborts on an unrelated dataset.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/management/commands/run_evals.py` around lines 86 - 91, The
handle validation guard must reject all invalid combinations: --save with
--case, --save with --dataset, and --case without --dataset. Update the guard
around the existing CommandError, and ensure the configs logic near line 106
uses only the single dataset when --case is set so _load_dataset does not
process unrelated datasets.

---

106-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

--case without --dataset aborts instead of running the case.

The case name is applied to every registered dataset, and _load_dataset raises CommandError for the first dataset that lacks it. Require --dataset when --case is given.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/management/commands/run_evals.py` at line 106, Update the
option validation in the run-evals command before the configs assignment to
reject --case unless --dataset is also provided, raising the command’s existing
CommandError with a clear message. Preserve the current behavior for
dataset-only and case-plus-dataset invocations, and prevent the case from being
applied across all REGISTRY entries.

src/backend/chat/evals/dashboard/template.html (1)

716-731: 📐 Maintainability & Code Quality | 🔵 Trivial

Nested ternaries in summaryRow/deltaHtml.

SonarCloud flags this construct for readability; consider extracting to an if/else chain or small helper for clarity.

♻️ Suggested refactor

-      const deltaHtml = pp == null
-        ? '<span class="muted">—</span>'
-        : pp === 0
-          ? '<span class="muted">=</span>'
-          : `<span class="${pp > 0 ? "delta-pos" : "delta-neg"}">${pp > 0 ? "+" : ""}${pp}pp</span>`;
+      let deltaHtml = '<span class="muted">—</span>';
+      if (pp === 0) {
+        deltaHtml = '<span class="muted">=</span>';
+      } else if (pp != null) {
+        deltaHtml = `<span class="${pp > 0 ? "delta-pos" : "delta-neg"}">${pp > 0 ? "+" : ""}${pp}pp</span>`;
+      }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/dashboard/template.html` around lines 716 - 731,
Refactor the nested ternary expressions in summaryRow, especially the deltaHtml
and fillB assignments, into clear if/else logic or a small local helper.
Preserve the existing null, zero, positive, and negative rate-difference outputs
and class assignments without changing behavior.

Source: Linters/SAST tools

.codespellrc (1)

14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider narrowing the skip to the data directories.

The whole chat/evals package is excluded, so Python sources and docstrings there lose spellchecking too. Skipping only ./src/backend/chat/evals/datasets and ./src/backend/chat/evals/baselines keeps coverage on code while ignoring the French fixtures.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.codespellrc at line 14, Update the codespell exclusions to remove the broad
./src/backend/chat/evals entry and instead skip only
./src/backend/chat/evals/datasets and ./src/backend/chat/evals/baselines,
preserving spellchecking for the package’s Python code and docstrings.

src/backend/.pylintrc (1)

22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Document why parallel Pylint was disabled.

Forcing jobs=1 slows lint across the whole backend, and the change is unrelated to the eval feature. If this works around a parallel-mode issue (e.g. pylint_django/similarity checker instability with the new modules), add an inline comment; otherwise consider reverting to jobs=0.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/.pylintrc` at line 22, Update the jobs setting in the Pylint
configuration to use parallel execution unless jobs=1 is required for a known
issue; if retaining jobs=1, add an inline comment documenting the specific
parallel-mode failure or dependency instability it avoids.

src/backend/chat/evals/evaluators/span.py (1)

21-22: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Guard against missing span-tree instrumentation.

ctx.span_tree raises SpanTreeRecordingError when the tree was not captured, so a single uninstrumented case turns each HasNoMatchingSpan did_not_call_* assertion into an evaluator error instead of a pass/fail result. Catch that exception and return a failed span-negative result before calling .any(...).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/evaluators/span.py` around lines 21 - 22, Update
HasNoMatchingSpan.evaluate to catch SpanTreeRecordingError when accessing
ctx.span_tree and return False before invoking any(self.query). Preserve the
existing negated any(...) behavior when span-tree instrumentation is available.

src/backend/chat/tests/evals/test_configs.py (1)

40-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test duplicates run_evals.py's dataset-loading logic instead of exercising it directly.

This re-implements _load_dataset's custom_evaluator_types assembly and Dataset.from_text call rather than calling shared logic, so the test can stay green even if the actual run_evals path diverges — undermining its own stated purpose of validating "the run_evals path".

Consider extracting the loading logic (e.g. EvalConfig.build_dataset(case_name=None) in configs/base.py) and having both run_evals.py and this test call it, so a regression in the real loading path is guaranteed to fail here too.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/tests/evals/test_configs.py` around lines 40 - 54, Extract
the shared dataset-construction logic from run_evals.py into an EvalConfig
method such as build_dataset(case_name=None) in configs/base.py, including
evaluator-type assembly and Dataset.from_text setup. Update run_evals.py and
test_all_datasets_parse_after_config_strip to call this method, while preserving
each caller’s existing name and case-selection behavior.

src/backend/chat/evals/production_agent.py (1)

102-174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider consolidating eval-vs-production wiring to reduce drift risk.

build_production_agent_service reimplements the web_search_enabled default logic (_is_web_search_enabled and _is_smart_search_enabled), and ensure_web_search_registered reimplements its own only_if_web_search_enabled prepare-gate and tool registration — both mirroring behavior that appears to live in AIAgentService._setup_web_search_tool/_run_agent. Since the whole point of this module is to test the real production wiring, any future change to that upstream logic won't automatically propagate here, silently making evals test stale behavior.

Consider exposing a small shared helper (e.g., a method or module-level function used by both AIAgentService and this eval harness) for the enablement decision and the stub-registration prepare-gate, so eval wiring can't drift from production wiring.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/production_agent.py` around lines 102 - 174,
Consolidate web-search enablement and registration logic shared by
AIAgentService and the eval harness into reusable helpers, then update
build_production_agent_service and ensure_web_search_registered to call them
instead of duplicating the _is_web_search_enabled/_is_smart_search_enabled
decision and only_if_web_search_enabled gate/tool setup. Keep production and
eval behavior identical while ensuring future changes to the shared wiring
propagate automatically.

src/backend/chat/evals/tool_stub_responses.py (1)

79-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider rejecting unknown keys in dataset tool_output JSON.

ToolStubResponses uses pydantic's default extra="ignore". A typo in a dataset's tool_output JSON key (e.g. misspelling web_search) would silently fall back to defaults instead of failing, making dataset authoring mistakes hard to catch.

♻️ Proposed fix

-class ToolStubResponses(BaseModel):
+class ToolStubResponses(BaseModel):
     """Per-case simulated tool payloads (JSON in dataset ``tool_output``)."""
+
+    model_config = ConfigDict(extra="forbid")
 
     web_search: dict[str, Any] | None = None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/evals/tool_stub_responses.py` around lines 79 - 86,
Configure the ToolStubResponses Pydantic model to reject unknown fields in
dataset tool_output JSON instead of silently ignoring them. Update the model
configuration near the existing fields while preserving all declared fields and
their defaults.

src/backend/chat/management/commands/create_eval_baseline.py (1)

30-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant resolve_run call.

set_baseline already calls resolve_run internally and raises the same FileNotFoundError, so Line 32 just re-reads and re-parses the run file.

♻️ Proposed cleanup

         try:
-            resolve_run(options["run"])
             baseline = set_baseline(

(drop resolve_run from the import as well)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/chat/management/commands/create_eval_baseline.py` around lines 30
- 39, Remove the redundant resolve_run(options["run"]) invocation from handle
and drop resolve_run from the imports. Keep set_baseline as the sole
run-resolution path while preserving the existing
FileNotFoundError-to-CommandError handling.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes